✨ Support `Parameter` in the CLI Option autocompletion functionality #1006

bckohan · 2024-09-29T21:28:28Z

Cf issue: #949

Note, this ~~includes (and supersedes) the changes from #974 and~~ implements these suggestions #949 (comment), with a few differences.

I attempted to push these changes to #974 but was asked to open a new PR here instead.

This PR does 3 things:

Allows autocompletion functions to accept a click.core.Parameter argument. I think this is necessary because without it you can't define generic completer functions that don't know what CLI options or arguments they may be attached to and that need to alter their behavior based on how click has processed the arguments before the incomplete argument. Concrete example: you have an option or argument that accepts multiple values but you do not wish those values to be repeated.
Allows CompletionItems to be returned in addition to strings and 2-tuples from autocompletion functions.
Fixes arg: edit this change was separated out in 🐛 Fix arg parameter of autocompletion #1134

A reusable completer function tutorial has been added to the autocompletion tutorials

svlandeg · 2024-10-25T09:11:27Z

Thanks for the PR! We'll get this reviewed and get back to you 🙏

svlandeg · 2024-10-25T09:18:12Z

@bckohan: the tests are failing because there isn't a 100% code coverage with the new commits. Could you look into this? 🙏

bckohan · 2024-11-08T23:26:31Z

@svlandeg I've updated the docs and added the necessary tests. Should be good to go!

svlandeg · 2024-11-13T10:40:41Z

Great, thanks! I've got this on my TODO list to review in detail, but realistically it might take me 1 or 2 more weeks to get to it 🙏

svlandeg · 2024-12-04T14:00:20Z

Ok so looking at the diffs of these two PRs (this one and #974) it appears that all the changes from #974 are still included here verbatim. As such I think it's actually easier to review these changes separately, i.e. one PR at a time. I've also pulled out the typo changes to avoid cluttering the diff: #1077 (I should have done that from the get-go).

@bckohan: do you see any reason not to merge #974 first, then continue working on the functionality from this PR?

bckohan · 2024-12-04T17:26:17Z

@svlandeg as long as the two PRs are merged in the same release that should be ok. Otherwise #949 alone will break downstream software.

tiangolo · 2024-12-04T18:53:37Z

@bckohan in which way would it break downstream software?

bckohan · 2024-12-04T18:57:55Z

@tiangolo Its not exactly broken - but deprecation warnings are being thrown with no recourse to fix them because autocompletion does not pass the Parameter objects. There are also downstream completers that expect to be able to pass a CompletionItem back because they're setting the type field.

tiangolo · 2024-12-04T23:21:02Z

I would think now there would be fewer warnings as the autocompletion parameter is now undeprecated. 😅

And the other shell_complete parameter was never documented, and the implementation was not completed, so I would expect people would not be using it yet. 🤔

There are also downstream completers that expect to be able to pass a CompletionItem back because they're setting the type field.

I'm not sure I got this, could you share an example?

It might also be useful to start in a GitHub Discussion with a minimal example we can copy paste that shows the problem (or if there are multiple problems, multiple examples).

bckohan · 2024-12-05T02:41:31Z

HI!

I think its reasonable to expect significant usage of shell_complete for the following reasons:

The deprecation message for autocompletion pointed to it.
It's documented by Click and was in the named parameter lists of Argument/Option.
Without the Parameter object there are things you simply can't do in generic completer functions (i.e. functions that are written without knowing which parameter they are going to be asked to complete).
It's much more reliable to fetch the parsed argument values for variadic arguments from the Context using the parameter name. The autocompletion interface as documented would have you manually parse the list of argument strings to determine if you need to exclude previously provided argument values, but see 5.
The argument list passed to autocompletion is usually empty because it's derived from ctx.args which only includes unparsed leftover arguments. This PR fixes the behavior to match the docs and also the test that is currently erroneously passing.

I'm extensively using shell_complete with Parameter objects downstream. And there's evidence here of others trying. I also added example/explanation of why this is needed to the tutorial in the docs. And here's real world production code where I'm using Parameter to filter out previously provided model objects for a generic model field completer.

CompletionItem is somewhat less important than all of the above. Click uses it because it's shell specific scripts are using inbuilt file/directory completer logic. To trigger these you have to be able to pass back meta info. I can think of 4 reasons to support this:

Click does, so there are likely generic completer functions in the wild that people might want to reuse on typer apps.
Its a trivial addition to the code.
Gives future implementors of other shells a hook to add meta information that may be useful for enabling whatever super awesome shell specific completer feature they want to use. The arbitrariness of type is important. It could be something other than file or directory.
Existing code is probably already passing these objects back (mine is at least!)

bckohan · 2025-01-24T23:31:12Z

I've separated out the fix to args here: #1134

github-actions · 2025-10-24T12:04:34Z

📝 Docs preview

Last commit 7734bcb at: https://bee8c5b3.typertiangolo.pages.dev

Modified Pages

https://bee8c5b3.typertiangolo.pages.dev/tutorial/options-autocompletion/ - (before)

svlandeg

This feature makes sense to me - e.g. being able to access param.name unlocks new functionality I can see being relevant to users.

I have some concerns about this part of the documentation historically using name as the parameter to be autocompleted. Because now using Parameter, we would go from accessing ctx.params.get("name") to ctx.params.get(param.name) with param.name being "name" in this particular case, which just adds unnecessary confusion at this point.

So, while I don't like adding more edits to a PR, I suggest to refactor all tutorial examples here to use user instead of name as the option we want to autocomplete. Just to 100% avoid this confusion.

I'll put this PR in draft and make those edits directly on the branch to then hopefully get this PR in a good state for merging.

docs/tutorial/options-autocompletion.md

…nto feat/autocompletion

docs/tutorial/options-autocompletion.md

svlandeg

Ok, even though this has exploded the diff, I really do think it's more clear to users this way, going from ctx.params.get("user") to ctx.params.get(param.name). I've reviewed everything in detail and think this looks good - I'm moving this to Tiangolo's internal TODO list to review.

Thanks again for all this work, @bckohan !